Conversation
| @@ -0,0 +1,9 @@ | |||
| pragma solidity ^0.4.18; | |||
There was a problem hiding this comment.
I made a change right before you put this PR up that removes the struct/library thing from StorageConsumer and StorageStateful. I think this change might be adding it back in?
If you rebase the latest from master, you won't need to add this BasicStorage contract since StorageConsumer is now simplified to only have a _storage address like this one has.
| import '../DelegatedStorage/DelegatedStorage.sol'; | ||
| import '../DelegatedStorage/KeyManagerInterface.sol'; | ||
| import '../Proxy/DelegateManagerInterface.sol'; | ||
| import 'zeppelin-solidity/contracts/ownership/Ownable.sol'; |
There was a problem hiding this comment.
nitpick alert ... external imports should be declared before locals
| import './BaseProxy.sol'; | ||
| import './DelegateManagerInterface.sol'; | ||
|
|
||
| contract DelegateManagable is BaseProxy { |
There was a problem hiding this comment.
thinking about having everything that extends Proxy be named with "Proxy" .. could call this ManagedProxy or something like that
| import '../Proxy/DelegateManagable.sol'; | ||
| import './KeyManagerInterface.sol'; | ||
|
|
||
| contract DelegatedStorage is BaseStorage, DelegateManagable { |
There was a problem hiding this comment.
thinking we should move everything that extends BaseStorage into the /Storage dir.
|
@cwhinfrey just a few comments. some organizational stuff we should think about changing, but we can play with it after it's merged |
|
forgot to 👍 |
2 participants
No description provided.